Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make html.Selection.map return []goja.Value instead of []string #2534

Merged
merged 1 commit into from
Jun 13, 2022

Conversation

oleiade
Copy link
Member

@oleiade oleiade commented May 13, 2022

This PR attempts to address #2471.

The existing Html.Selection.map function would return a []string under the hood. This behavior would result in effectively obfuscating any type that's not a string during the parsing of an HTML or XML document. This PR modifies the behavior and signature of the function to fit what we have documented. The return value should be an Array of values on the JS script side, a []goja.Value in the Go implementation of the library.

The reason why we returned []string in the first place is because goquery, the library we depend on to perform HTML/XML parsing, own map method does so. To work around the limitation, I adopted an approach consisting in serializing the values with map over to JSON, and deserialize them before we return from the Map function.

It works, but I'm open to any other approach that might be simpler or more performant 🙇🏻

@oleiade oleiade requested review from mstoykov and olegbespalov May 13, 2022 09:43
@oleiade oleiade self-assigned this May 13, 2022
@oleiade oleiade added the bug label May 13, 2022
@oleiade oleiade added this to the v0.39.0 milestone May 13, 2022
olegbespalov
olegbespalov previously approved these changes May 17, 2022
Copy link
Contributor

@olegbespalov olegbespalov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@oleiade
Copy link
Member Author

oleiade commented May 18, 2022

Here's an example script demonstrating it at play

import { parseHTML } from 'k6/html'

const data = `
<ListAllMyBucketsResult>
   <Buckets>
      <Bucket>
         <CreationDate>myCreationDateTimestamp</CreationDate>
         <Name>myBucketName</Name>
      </Bucket>
   </Buckets>
   <Owner>
      <DisplayName>string</DisplayName>
      <ID>string</ID>
   </Owner>
</ListAllMyBucketsResult>`

export default function () {
    const buckets = parseHTML(data)
        .find('Buckets')
        .children()
        .map(function (_, bucket) {
            let bucketObj = {}
            bucket.children().each(function (_, elem) {
                switch (elem.nodeName()) {
                    case 'name':
                        Object.assign(bucketObj, { name: elem.textContent() })
                        break
                    case 'creationdate':
                        Object.assign(bucketObj, { creationDate: elem.textContent() })
                        break
                }
            })

            return bucketObj
        })

    console.log(JSON.stringify(buckets)) // Prints [{"creationDate":"timestamp","name":"string"}]
    console.log(buckets[0].name) // Prints "myBucketName"
    console.log(buckets[0].creationDate) // Prints "myCreationDateTimestamp"
}

cc @mstoykov

@oleiade oleiade force-pushed the fix/html_parse branch 2 times, most recently from b7df16c to 82118c6 Compare June 9, 2022 09:06
js/modules/k6/html/html_test.go Outdated Show resolved Hide resolved
@oleiade oleiade requested a review from mstoykov June 10, 2022 09:38
@oleiade oleiade merged commit a983e27 into master Jun 13, 2022
@oleiade oleiade deleted the fix/html_parse branch June 13, 2022 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

k6/html Selection.map produces an array of strings
3 participants